Skip to content

refactor: use core hw activities#1044

Open
ovitrif wants to merge 19 commits into
masterfrom
refactor/core-hww-activities-1029
Open

refactor: use core hw activities#1044
ovitrif wants to merge 19 commits into
masterfrom
refactor/core-hww-activities-1029

Conversation

@ovitrif

@ovitrif ovitrif commented Jun 24, 2026

Copy link
Copy Markdown
Collaborator

Closes #1029
Refs #998

Description

This PR:

  1. Replaces separate in-memory Trezor transaction handling with wallet-scoped activities and transaction details persisted by bitkit-core 0.4.0.
  2. Derives stable hardware wallet IDs through bitkit-core using enum-backed wallet types, then preserves that wallet scope across watcher events, activity reads, tags, details, seen state, boost lookups, filters, navigation, and Explore.
  3. Reconciles each full watcher snapshot with Core before publishing UI state, preserves transfer metadata, and serializes watcher start, persistence, removal, and purge so stale or removed wallet rows cannot return.
  4. Keeps transfer-to-spending as one correctly labelled hardware-wallet activity from pending through confirmation, including when another wallet contains the same transaction ID.
  5. Purges wallet-scoped activity data when a paired device is removed and rebuilds its history after re-pairing.
  6. Retains the current Trezor busy, retry, and cancellation behavior from master and allows on-device confirmations to complete within the Bridge signing timeout.

Preview

No visual redesign. Existing Home, Activity Detail, tag filtering, and Explore surfaces now render wallet-scoped hardware activities from bitkit-core.

QA Notes

Manual Tests

  • 1. Settings → Hardware Wallets → pair a Trezor with a seed distinct from the app wallet: the connected hardware wallet appears on Home.
  • 2. Fund at least two hardware wallet addresses with different amounts → mine a block: Home shows one blue hardware activity for each transaction.
  • 3. Home → Show All → switch Received, Sent, and All tabs: each hardware activity appears only in the matching views and ordering remains correct alongside app activities.
  • 4. Hardware activity → Activity Detail → add a tag → back → filter by that tag: the scoped activity remains visible; Explore shows its transaction ID, inputs, and outputs.
  • 5. Transfer Funds → enter an amount → approve Recipient, Amount, Locktime, and Summary on the Trezor within 120 seconds → immediately tap Continue Using Bitkit within five seconds: Home shows one pending blue Transfer / From Savings activity.
  • 6. Pending transfer → Activity Detail → Blocktank Regtest → mine one block → reopen Activity Detail: the same row is confirmed and Spending increased by the transferred amount.
  • 7. Settings → Hardware Wallets → remove the paired device: its activities disappear; re-pairing rebuilds its confirmed history without duplicate rows.
  • 8. regression: app on-chain and LN activities → Activity Detail, tags, filters, and Explore: behavior matches master.

Automated Checks

  • Unit tests modified: HwWalletRepoTest.kt, ActivityRepoTest.kt, and TransferRepoTest.kt cover wallet-scoped watcher ownership, canonical persistence, persistence recovery, watcher/removal races, transfer reconciliation, and removal purge behavior.
  • Unit tests modified: TrezorRepoTest.kt, TrezorBridgeTransportTest.kt, TrezorViewModelTest.kt, and TrezorExceptionExtTest.kt cover Core-derived identity and Trezor transport, retry, busy, cancellation, and signing behavior.
  • Unit tests modified: ActivityListViewModelTest.kt, ActivityDetailViewModelTest.kt, TransferViewModelTest.kt, and AppViewModelSendFlowTest.kt cover wallet-scoped filtering, boost relationships, detail routing, tags, and transfer metadata.
  • Hardware wallet journeys updated: activity-blue-icons.xml, activity-detail-hw-tags.xml, transfer-to-spending.xml, and its fee/warmup variants cover multiple transactions, hardware activity UI, the complete signing sequence, pending state, same-backend mining, and confirmed state.
  • Product-level verification: the Bridge-backed Trezor emulator exposed multiple hardware activities, reconciled stale rows from a full watcher snapshot, persisted a hardware tag, rendered Explore inputs/outputs, signed Transfer To Spending, showed exactly one pending hardware transfer before mining, and kept that same wallet-scoped row confirmed after mining without a default-wallet duplicate.
  • Local verification: just compile, just test, and just lint passed; all hardware-wallet journey XML files passed xmllint.

@greptile-apps

This comment has been minimized.

greptile-apps[bot]

This comment was marked as resolved.

@ovitrif ovitrif self-assigned this Jun 24, 2026
chatgpt-codex-connector[bot]

This comment was marked as resolved.

@ovitrif ovitrif marked this pull request as draft June 24, 2026 22:22
@ovitrif ovitrif changed the title feat: integrate hardware wallet activities with core refactor: use scoped hw activities Jun 24, 2026
@ovitrif ovitrif changed the title refactor: use scoped hw activities refactor: use core hw-scoped activities Jun 25, 2026
@ovitrif ovitrif changed the title refactor: use core hw-scoped activities refactor: use core hw activities Jun 25, 2026
@ovitrif

This comment was marked as outdated.

@ovitrif ovitrif force-pushed the refactor/core-hww-activities-1029 branch 2 times, most recently from f3f9a2c to 435d9b0 Compare July 4, 2026 19:35
@ovitrif

ovitrif commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Status update:

  • This PR now consumes bitkit-core 0.3.9.
  • Local verification passed on c0274e311: just compile, just test, and just lint.
  • There are no unresolved review threads currently open.
  • Coordination note: feat: bump core to 0.3.9 and hw reliability #1062 touches much of the same Core/HW wallet surface. I’d recommend keeping this PR as the base for the Core hardware-activity refactor, then rebasing or splitting feat: bump core to 0.3.9 and hw reliability #1062 afterward so the hardware reliability fixes stay separately reviewable.

@ovitrif

ovitrif commented Jul 5, 2026

Copy link
Copy Markdown
Collaborator Author

Status update after HW journey validation:

  • The core unblock is resolved for this branch: it consumes bitkit-core 0.3.9 and the Android artifact from the publish task.
  • I re-ran the Bridge Trezor emulator flows on 8372a730f: connect-home-tile.xml, activity-blue-icons.xml, activity-detail-hw-tags.xml, and detail-overview.xml.
  • Validation passed for the important HW activity/wallet paths: deterministic Trezor pair/fund, blue Core-backed activity rows, All Activity tabs, persisted hwtest tag, tag filter, Explore txid/input/output data, hardware wallet detail, transfer amount screen after the first-run intro, remove cancel, final remove, watcher stop, and hardware activity purge.
  • The app log confirms cleanup after removal: Deleted '1' activities for hardware wallet ....
  • I also updated the HW transfer journey specs so fresh emulator runs handle the first-run Transfer To Spending intro before asserting HardwareTransferAmount; otherwise the journey can false-fail before reaching the amount screen.
  • No fatal/crash signatures showed up in logcat. Existing local checks remain: just compile, just test, and just lint passed on c0274e311; xmllint passed for the updated HW journey XMLs on 8372a730f.

Remaining handoff: wait for checks/reviews on 8372a730f. If those settle cleanly, this should be ready to move out of draft/request review. #1062 overlaps with this work and should likely be rebased or split after this lands, but treat that as coordination guidance rather than a blocker for this branch.

@piotr-iohk

Copy link
Copy Markdown
Collaborator

Trezor Android: #1044 vs #1062 — QA findings

Date: 2026-07-06
Tested: refactor/core-hww-activities-1029 (#1044) on physical device (to.bitkit.dev)
Comparison: feat/wallet-scoped-core-0.3.9 (#1062) shows correct Trezor balance on same device

Scope mismatch

#1044 may be a stretch for 2.4.0: iOS Trezor is still behind, and for cross-platform consistency we should not ship HW tags or Explore inputs/outputs in 2.4.0. That fits 2.5.0 / #1029 instead.

Finding 1: Trezor shows 0 balance + empty activity list (#1044)

Symptom: Paired Trezor with known funds/history → app shows 0 sats and no activities.

Not a Trezor/connect issue. Watcher starts; Core emits data; UI never updates.

Cause: In HwWalletRepo.observeWatcherEvents(), persistHardwareActivities() runs before _watcherData is updated. On failure it return@collect — balance and activities never reach UI.

Log evidence (bitkit_2026-07-06_08-34-36.log):
bitkit_logs_2026-07-06_08-50-23.zip

Started watcher 'ble:5B:B1:C8:E9:4C:BE|nativeSegwit'
ERROR Failed to persist hardware activities
  AppError='Failed to upsert onchain_activity: UNIQUE constraint failed: onchain_activity.id'
hardwareWallets=[{"id":"ble:5B:B1:C8:E9:4C:BE","sats":0}]

Likely persist root cause: HW activity ids = txids. Error on onchain_activity.id suggests stale pre-0.3.9 schema (id globally unique) or failed migration — collides when same tx exists on Bitkit wallet and Trezor (e.g. transfer between wallets). Fresh install / clear app data should confirm.

#1062: Updates _watcherData from watcher first; does not gate UI on persist. Works on same device.

Finding 2: Transfer shows duplicate rows — Transfer + Sent (#1044)

On transfer to spending, activity list shows two rows for one flow:

  • Transfer (Bitkit wallet)
  • Sent (Trezor / HW wallet)

Both appear in unified list; both are taggable. Not a quick polish issue — needs product rules for cross-wallet dedup/linking (transfer_tx_id, filtering, etc.). #1044 merges dupes across address-type watchers, not Bitkit↔Trezor transfer pairs.

Finding 3: Scope ahead of 2.4.0 (#1044)

#1044 adds:

  • Tag support on HW activities
  • Explore inputs/outputs
  • Boost with wallet-scoped Core lookup
  • HW activities in global Activity tab (filters, tags, blue icons)

Reasonable for 2.5.0 / #1029, but not aligned with a minimal, iOS-consistent 2.4.0 Trezor release.

Prod / migration concern

Not a prod issue today — no Trezor in prod.

Before HW launch (2.5.0): migration must work on upgrade; UI must not depend on persist succeeding; transfer dedup needs design.

Clear app data is a dev QA workaround, not a ship strategy.

Recommendation

Release PR Action
2.4.0 Trezor v1 #1062 Ship — minimal scope, works on device, #1030 reliability
2.5.0 #1044 Keep draft — fix persist gate, migration/collision, transfer dedup, then tags/Explore/Boost QA

Later: cherry-pick #1062 reliability commits onto #1044 for 2.5.0; don’t merge as competing 0.3.9 paths.

#1044 fixes needed (2.5.0, not blocking v1)

  1. Decouple UI from persist — update watcher state even if Core persist fails; log error separately.
  2. Fix persist failure — ensure 0.3.9 (wallet_id, id) migration on upgrade; handle shared txids across wallets.
  3. Transfer dedup UX — one logical row per transfer, not Transfer + Sent both taggable.
  4. Continue Bitkit Core support for hardware wallet activities #1029 scope (tags, Explore, Boost) only after above is stable.

Links

@ovitrif

ovitrif commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Decouple UI from persist — update watcher state even if Core persist fails; log error separately.

Addressed this Android-side gating issue: watcher state now updates before Core persistence, so the Trezor balance/activity UI stays live even if a Core upsert fails. Persistence failures are still logged with the watcher id, and received-transaction emission remains behind successful persistence.

Validation: just compile, just test, just lint.

The migration/collision and transfer dedup points remain broader draft follow-ups outside this scoped fix.

Resolved in f546f58

@ovitrif ovitrif added this to the 2.5.0 milestone Jul 6, 2026
@ovitrif

ovitrif commented Jul 6, 2026

Copy link
Copy Markdown
Collaborator Author

Set milestone to 2.5.0 for clarity 👍🏻 , thanks for review & checks @piotr-iohk 🙏🏻

github-advanced-security[bot]

This comment was marked as resolved.

@ovitrif ovitrif changed the base branch from master to feat/wallet-scoped-core-0.3.9 July 7, 2026 12:13
@ovitrif ovitrif force-pushed the refactor/core-hww-activities-1029 branch from 708cd21 to 794722a Compare July 7, 2026 12:13
Base automatically changed from feat/wallet-scoped-core-0.3.9 to master July 9, 2026 08:53
@ovitrif ovitrif force-pushed the refactor/core-hww-activities-1029 branch from ea9a252 to 5ae816a Compare July 9, 2026 20:35
@ovitrif

ovitrif commented Jul 9, 2026

Copy link
Copy Markdown
Collaborator Author

Trezor shows 0 balance + empty activity list

Transfer shows duplicate rows - Transfer + Sent

Scope ahead of 2.4.0

The branch is now rebased on the merged #1062 and latest master, and consumes bitkit-core 0.4.0. The old pre-release database state from this report is no longer the branch/base under review. Wallet-scoped persistence is the source of truth, and watcher state is published only after the matching activities and transaction details are available for list/detail navigation.

I reran the full Bridge flow on the current head with two independently funded and mined hardware receives. Both appeared once with the correct wallet scope, and list tabs, tags, tag filtering, Activity Detail, and Explore all used the persisted entries.

Transfer-to-spending now reconciles the watcher-created sent activity with the transfer metadata. The Bridge run showed one pending Transfer / From Savings row, then the same row confirmed after mining while Spending increased by the transferred amount. Device removal purged that wallet's activities, and re-pairing rebuilt its confirmed history.

This work remains scoped to 2.5.0 / #1029. just compile, just test, and just lint all pass on the current head.

Resolved in 5ae816a

@ovitrif ovitrif marked this pull request as ready for review July 9, 2026 20:39
@chatgpt-codex-connector

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bitkit Core support for hardware wallet activities

4 participants